Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add additional parameters to ContentsUploadModal to be reusable in different scenarios #5881

Merged
merged 10 commits into from
Mar 31, 2024

Conversation

erral
Copy link
Member

@erral erral commented Mar 14, 2024

In a recent project I have found ContentsUploadModal very useful, and I have copied it over to provide the end-user with file upload capabilities.

But as I wanted to restrict its use (max file size, only allow images, ...) and I have seen that react-dropzone's <Dropzone /> allows extra parameters, I have added the option to ContentsUploadModal to pass additional parameters to Dropzone.

This way, one can simply reuse the modal component, which is great.

Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit d38b4fd
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/660853b2d0d3bf000851168c

Copy link

netlify bot commented Mar 14, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit d38b4fd
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/660853b268641c0008649937

@erral erral changed the title Make ContentsUploadModal reusable Add additional parameters to ContentsUploadModal to be reusable in different scenarios Mar 14, 2024
Comment on lines 225 to 239
if (multiple) {
dropzoneOptions['multiple'] = multiple;
}
if (minSize) {
dropzoneOptions['minSize'] = minSize;
}
if (maxSize) {
dropzoneOptions['maxSize'] = maxSize;
}
if (accept) {
dropzoneOptions['accept'] = accept;
}
if (disabled) {
dropzoneOptions['disabled'] = disabled;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (multiple) {
dropzoneOptions['multiple'] = multiple;
}
if (minSize) {
dropzoneOptions['minSize'] = minSize;
}
if (maxSize) {
dropzoneOptions['maxSize'] = maxSize;
}
if (accept) {
dropzoneOptions['accept'] = accept;
}
if (disabled) {
dropzoneOptions['disabled'] = disabled;
}
const dropzoneOptions = {
multiple,
minSize,
maxSize,
accept,
disabled,
};

@erral you could simplify this code as such and you wouldn't need to have all of these if statements.
I assume that the drag and drop library would know to handle even the options that have null values

Copy link
Member Author

@erral erral Mar 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @ichim-david surely it will. I am still a react novice for such structures and still go for if-like thingies. 😅 I will check if the library handles the null values correctly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have checked this and indeed the plugin handless the null values correctly, so I have updated the PR. Thanks again for the review.

@ichim-david ichim-david self-requested a review March 17, 2024 19:36
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed only the change log entry.

packages/volto/news/5881.feature Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve of news item. Still needs approval from @plone/volto-team.

@stevepiercy stevepiercy requested a review from a team March 29, 2024 14:31
@sneridagh sneridagh merged commit 6d50896 into main Mar 31, 2024
73 checks passed
@sneridagh sneridagh deleted the erral-reusable-content-upload branch March 31, 2024 11:49
sneridagh added a commit that referenced this pull request Apr 16, 2024
* main: (28 commits)
  Bump vite from 5.1.5 to 5.1.7 (#5946)
  Fix pt_BR translation of invalid email message (#5953)
  Fix markup shortcuts for bold, italic and strikethough fix (#5606)
  Release 18.0.0-alpha.27
  Release @plone/types 1.0.0-alpha.10
  Improve color widget picker and types (#5948)
  Enhanced navigation reducer in Volto (#5817)
  Release 18.0.0-alpha.26
  Rename news item
  Release @plone/slate 18.0.0-alpha.11
  Release @plone/registry 1.5.5
  Release @plone/types 1.0.0-alpha.9
  docs: Cleanup obsolete EEA projects and update info about EEA main website (#5943)
  Bump vite from 5.1.4 to 5.1.5 (#5942)
  Add a new label `needs: triage` to new bug reports (#5940)
  Fix redirect of `https://sustainability.eionet.europa.eu` to `https:/… (#5941)
  Does not show borders in addon block inputs (#5898)
  Fix edge case in search options mangling when the options are false-ish (#5869)
  Add additional parameters to ContentsUploadModal to be reusable in different scenarios (#5881)
  fix(slate): fix insert/remove element edgecase bug in slate (#5926)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants